TIKA-4856: /unpack/thumbnail returns the document thumbnail with its metadata - #3096
TIKA-4856: /unpack/thumbnail returns the document thumbnail with its metadata#3096dschmidt wants to merge 24 commits into
Conversation
… metadata Parses in unpack mode with a fixed configuration (the first PDF page and EMF/WMF images rendered, THUMBNAIL and RENDERING embedded documents extracted with their metadata), then picks the raster THUMBNAIL directly below the document, the rendering of a vector THUMBNAIL, or the RENDERING of the first page, and answers with JSON: the embedded document's metadata and the image as base64. 204 when the document has no thumbnail.
|
Remarked it as draft as I'm wondering if we need a better concept |
…unpack, thumbnail-defaults in the server config, PDF maxRenderedPages
# Conflicts: # CHANGES.txt
There was a problem hiding this comment.
Pull request overview
Adds first-class thumbnail support to Tika Server by centralizing “thumbnail defaults” (PDF first-page rendering + EMF/WMF thumbnail rendering) and exposing them via query flags and a new convenience endpoint, while also introducing a PDF parser limit to bound rendering without limiting text extraction.
Changes:
- Introduces
ThumbnailDefaultsandThumbnailSelector, enabling consistent server-side thumbnail rendering/selection behavior. - Adds
?renderThumbnails=trueto/rmeta,/unpack, and/unpack/all, plus a new/unpack/thumbnailendpoint returning{metadata, image(base64)}. - Adds
PDFParserConfig.maxRenderedPagesand enforces it in both PDF rendering paths, with new tests.
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tika-server/tika-server-standard/src/test/java/org/apache/tika/server/standard/UnpackerThumbnailTest.java | End-to-end tests for /unpack/thumbnail and /rmeta?renderThumbnails=true. |
| tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/resource/ThumbnailSelectorTest.java | Unit tests for thumbnail selection precedence rules. |
| tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/resource/ThumbnailDefaultsTest.java | Unit tests for built-in defaults, config overrides, and merge behavior. |
| tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/UnpackerResource.java | Adds /unpack/thumbnail and renderThumbnails query param support for unpack endpoints. |
| tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/TikaResource.java | Wires ThumbnailDefaults from server config and exposes them to resources. |
| tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/ThumbnailSelector.java | Implements thumbnail selection logic among embedded docs (thumbnail vs rendering fallback). |
| tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/ThumbnailDefaults.java | Defines built-in thumbnail parser JSON defaults + config override/merge/application logic. |
| tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/RecursiveMetadataResource.java | Adds renderThumbnails query param to /rmeta endpoints and applies defaults. |
| tika-serialization/src/main/java/org/apache/tika/config/loader/TikaJsonConfig.java | Adds thumbnail-defaults as a known top-level JSON config key. |
| tika-parsers/.../src/test/java/org/apache/tika/parser/pdf/PDFMaxRenderedPagesTest.java | Tests for maxRenderedPages behavior and JSON config support. |
| tika-parsers/.../src/main/java/org/apache/tika/parser/pdf/PDFParserConfig.java | Adds maxRenderedPages with validation and accessor methods. |
| tika-parsers/.../src/main/java/org/apache/tika/parser/pdf/PDFParser.java | Uses maxRenderedPages to bound rendered page range for PDF rendering. |
| tika-parsers/.../src/main/java/org/apache/tika/parser/pdf/PDF2XHTML.java | Skips per-page rendering after maxRenderedPages for page-end rendering strategy. |
| docs/modules/ROOT/pages/using-tika/server/index.adoc | Documents thumbnail behavior, query flag, endpoint, and config override block. |
| CHANGES.txt | Adds release notes for the new thumbnail capabilities and maxRenderedPages. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Mh, maybe the /thumbnail endpoint should have the renderThumbnails param as well. So we can still opt in to rendering while using the selection logic anyhow. So it's a simple consistent switch for pure extraction or more expensive rendering |
…s opt-in everywhere
|
Done: /unpack/thumbnail takes renderThumbnails as well. Without it a request is the cheap extraction of stored thumbnails, rendering is opt-in with the same switch everywhere. |
# Conflicts: # CHANGES.txt
…are detected by content since TIKA-4861
# Conflicts: # CHANGES.txt
# Conflicts: # CHANGES.txt
# Conflicts: # CHANGES.txt
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Copilot reviewed 15 out of 17 changed files in this pull request and generated 5 comments.
…entry, clean up after the test - ThumbnailDefaults.with(ThumbnailDefaults) serialized each component to a JSON string and parsed it back, once per component and per request; both overloads now merge the nodes, and a component that only the other side has is copied rather than shared. - the loop that maps the selected metadata back to its zip entry kept scanning after it had found it. - the test left its unpack directory behind, where UnpackerResourceTest deletes its own.
There was a problem hiding this comment.
🔵 Needs a closer look
It changes server API behavior and parsing/rendering controls across multiple modules (server + PDF parser), warranting final human validation despite only minor actionable feedback.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
tika-server/tika-server-standard/src/test/java/org/apache/tika/server/standard/UnpackerThumbnailTest.java:203
- The test asserts
response.getMediaType().toString()equals exactlyapplication/json, which can be brittle becausetoString()may include parameters (e.g., a charset) depending on the JAX-RS provider/CXF behavior. Other server tests typically assert on type/subtype instead, which avoids false failures when parameters are present.
- Files reviewed: 15/17 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
Argh. I'm sorry. I thought I posted a review yesterday. Something went wrong. I'm hesitant to add a special endpoint/parameter for thumbnails. I asked 🤖 to come up with a plan to get you what you want largely with what exists and some of what you've added. Let me know what you think about this. Also, fellow devs, please join in the conversation. This is what it came up with: |
|
Just to make sure I get it right: It's exactly what I wanted to avoid to be honest - it doesn't make sense as global config (in the metadata request I want OCR, in the thumbnails request I don't), I don't want to hardcode this in my application and I don't want to make it config in my application. The idea is to make it easy to retrieve thumbnails (as part of a metadata request and also as a standalone "just give me a thumbnail, I don't care"). In my use case (OpenCloud) I want to support previews for as many file formats as possible and Tika itself will always know better what it supports than any third party consumer. Moreover, I would really like to avoid having to maintain configs for different Tika versions, Maybe we can have some kind of preset functionality? P.S.: yes, of course we can split maxRenderedPages out of this PR. |
|
This would leave client side to arbitrate thumbnail vs rendering. This is an option to leave just one per: |
|
Let me think some more.... |
|
Like this? |
|
Pretty much, yeah! :) tbd what should be part of which preset (e.g. how comes disabling OCR into play) but yeah - I want to develop against an abstract concept, not against a gazillion of granular settings |
|
This would also allow initialization/baked in settings so that we could still prevent actual users from modifying settings outside of the allowed set of presets. Do we allow one preset per call, rather than the array? More than one gets confusing to me on precedence. |
|
Not sure, I think it could be documented in which order they are merged and how (deep-merging objects is usually clear, arrays maybe also merged, unfortunately means they cant be reset) Certainly easier documentation than documenting all the neccessary knobs to get all possible thumbnails :) |
|
Then again: a single preset is already much better than no preset support at all :) |
|
Let's start with a single preset allowed and no customizations. Users have a preset or customization or nothing. We can add complexity if necessary later? |
|
so something like this: ? Or would you go with singular form "preset" from the beginning on? |
|
We should keep How about: |
|
I'm really worried about debugging precedence with presets AND config, and then also with multiple presets. Will a single preset be enough? |
|
Not sure I would want to put that into stone, but - as I said - a single preset is already much better than no preset ... sooo ... 😅 |
|
If a user can tweak a preset or select multiple presets, they can send a config... But seriously, this is all off the top of my head. Let me know what you think. |
This is a bit awkward to me to be honest, I would prefer the preset in body or as query param behind ? - not in the url like this, that's pretty inflexible and doesnt work with other params at the same time.. apart from that ... I don't care too much tbh. I just care about the outcome (easy thumbnail access :)) |
|
Would you be able/willing to work on this? I think you have stronger opinions than I have (and know Tika a lot better obviously) and it might be easier for you to steer your 🤖 directly than for you to steer my 🤖 via me as proxy |
|
LOL..., sure. Apologies, but I"m going to start with: |
|
ok, boss 😁 |
Proof of concept for TIKA-4856, restructured after the discussion in the ticket. The core is the thumbnail defaults and the switch that applies them to the existing endpoints; that is what a client needs to get a document's thumbnail without knowing parser component names. The
/unpack/thumbnailendpoint is a convenience on top and can be dropped or reshaped without losing the rest. The shape of the switch (query parameter) and of the config block (top-level key) are open to change.Why
The thumbnail of a document is one of its embedded documents, typed
THUMBNAIL, and/rmetalists it already. Where a raster image only exists after rendering (the first page of a PDF, the EMF/WMF thumbnail of an Office document), a request needs a parse context that knows the component names, and that context should be the same for every caller.Thumbnail defaults
ThumbnailDefaults(tika-server-core) holds that context in one place: first PDF page rendered at 96 dpi (maxRenderedPages: 1, so the text is still extracted from the whole document), EMF/WMF thumbnail rendered (that one only,renderOnlyEmbeddedResourceTypes: ["THUMBNAIL"]from #3095).Three layers, each overriding the one before: built-in defaults, a
thumbnail-defaultsblock in the server config (same shape as a request config), the request's ownconfigpart. The defaults are plain JSON parser configurations set withParseContext.setJsonConfig, so a parser that is not installed never reads them.?renderThumbnails=trueOn
/rmeta,/unpack,/unpack/alland/unpack/thumbnail: lays the defaults under the request. Without it a parse is the cheap extraction of stored thumbnails; rendering is opt-in, with the same switch everywhere. The index request of a search service becomes/rmeta/text?renderThumbnails=true: metadata, text, and the thumbnail with its dimensions in one parse./unpack/thumbnail(optional)What only makes sense when the thumbnail is all the caller wants (no text extraction, no OCR, only THUMBNAIL and RENDERING extracted, image capped at 32 MiB), plus the defaults when
renderThumbnails=true; a PDF without the switch answers 204.ThumbnailSelectorpicks the raster THUMBNAIL below the document, the rendering below a vector THUMBNAIL, or the first page RENDERING, and answers as JSON:PDFParserConfig.maxRenderedPagesBounds the page rendering of both rendering strategies independently of
maxPages; without it/rmetacould only render the first page by also cutting the text after it.Verified
Against a server built from main plus the open thumbnail PRs: docx, xlsx, doc, xls, ppt, pptx, odt, epub, GeoGebra, Pages, Numbers, Keynote, mp3, m4a, flac, ogg, pdf, nef and pef answer with the right image; a zip, a plain jpeg and a doc without a thumbnail answer 204. Raw camera files no longer need their file name since #3099.
Open points
thumbnail-defaultsis a top-level config key, which meant adding it to the known keys inTikaJsonConfig(tika-serialization). Putting it underserverwould avoid that at the cost of a field in the server config class; fine either way.embedded-limits.maxDepthis 3 in the fixed context because of TIKA-4857, 2 once that is fixed.https://issues.apache.org/jira/browse/TIKA-4856